Skip to content

Restore error code from DecodeGeneralName#10793

Merged
douzzer merged 1 commit into
wolfSSL:masterfrom
embhorn:gh10790
Jul 1, 2026
Merged

Restore error code from DecodeGeneralName#10793
douzzer merged 1 commit into
wolfSSL:masterfrom
embhorn:gh10790

Conversation

@embhorn

@embhorn embhorn commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Description

PR #10279 added DecodeGeneralNameCheckChars, which returns ASN_PARSE_E for an embedded NUL in dNSName/rfc822Name/URI SANs, aborting the whole certificate parse before hostname matching ever runs. This broke cert testing in curl and changed the expected failure mode.

Fixes #10790

Dependency on fix in Python OSP wolfSSL/osp#347

Testing

  • tests/api/test_asn.c — updated the NUL sub-case: the cert now must parse (was ASN_PARSE_E), with the entry stored and the embedded NUL preserved (length 4, not truncated).
  • tests/api/test_ossl_x509.c — new test_wolfSSL_X509_check_host_embedded_nul_san: the exact curl-311 scenario (CN=localhost, SAN=localhost\0h). Pins that the cert parses and the host is still rejected (correct CN must not rescue the wrong SAN).

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@embhorn embhorn self-assigned this Jun 26, 2026
@embhorn embhorn mentioned this pull request Jun 26, 2026
Closed
Copilot AI reviewed Jun 26, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts X.509 SubjectAltName parsing so embedded NUL bytes in IA5String-based GeneralNames (dNSName/rfc822Name/URI) do not abort certificate parsing with ASN_PARSE_E, restoring the expected verification-time failure mode (e.g., DOMAIN_NAME_MISMATCH) and fixing the curl test 311 regression described in #10790.

Changes:

  • Stop rejecting embedded-NUL IA5String SAN values at parse time by removing DecodeGeneralNameCheckChars from both ASN parsers.
  • Add/adjust tests to ensure certificates with embedded-NUL dNSName SANs still parse and store the full (non-truncated) value, while hostname verification still fails.
  • Update doxygen to warn that wolfSSL_X509_get_next_altname() returns a C string without a length and can be silently truncated by embedded NULs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
wolfcrypt/src/asn.c Removes parse-time NUL rejection for GeneralName IA5String SANs; documents rationale in-code.
wolfcrypt/src/asn_orig.c Mirrors the same behavior change in the legacy ASN parser.
tests/api/test_asn.c Updates SAN tests to require parse success and verify embedded NUL is preserved in stored altName data.
tests/api/test_ossl_x509.h Registers the new regression test in the ossl_x509 test group.
tests/api/test_ossl_x509.c Adds regression test reproducing curl-311 scenario (good CN, bad SAN with embedded NUL).
doc/dox_comments/header_files/ssl.h Adds API documentation warning about embedded NUL truncation with wolfSSL_X509_get_next_altname().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wolfSSL-Fenrir-bot
wolfSSL-Fenrir-bot reviewed Jun 26, 2026
View reviewed changes

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10793

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

@embhorn
embhorn marked this pull request as ready for review June 26, 2026 16:34
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

retest this please

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m0plus

  • FLASH: .text -56 B (-0.1%, 63,439 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m3

  • FLASH: .text -56 B (-0.0%, 121,353 B / 262,144 B, total: 46% used)

gcc-arm-cortex-m4

  • FLASH: .text -64 B (-0.0%, 198,988 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text -64 B (-0.1%, 65,995 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .text -64 B (-0.0%, 173,610 B / 262,144 B, total: 66% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text -64 B (-0.0%, 179,736 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-min-ecc

  • FLASH: .text -64 B (-0.1%, 60,973 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text -64 B (-0.0%, 768,068 B / 1,048,576 B, total: 73% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .text -64 B (-0.0%, 211,309 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text -64 B (-0.0%, 323,408 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-sp-math

  • FLASH: .text -64 B (-0.1%, 60,973 B / 262,144 B, total: 23% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text -64 B (-0.1%, 122,125 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text -64 B (-0.0%, 234,686 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

  • FLASH: .text -64 B (-0.0%, 198,988 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text -64 B (-0.0%, 234,686 B / 262,144 B, total: 90% used)

linuxkm-pie

  • Data: __patchable_function_entries -8 B (-0.0%, 24,280 B)

stm32-sim-stm32h753

@embhorn embhorn mentioned this pull request Jun 26, 2026
Merged
@vszakats vszakats mentioned this pull request Jun 28, 2026
Closed
1 task
@embhorn

embhorn commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

retest this please

@embhorn embhorn assigned wolfSSL-Bot and unassigned embhorn Jun 30, 2026
@embhorn

embhorn commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

@wolfSSL-Bot - please review / merge OSP fix prior to this fix
wolfSSL/osp#347

@douzzer
douzzer merged commit 7dd269f into wolfSSL:master Jul 1, 2026
431 of 435 checks passed
vszakats added a commit to curl/curl that referenced this pull request Jul 2, 2026
@vszakats
GHA: update dependency wolfSSL/wolfssl to v5.9.2, apply upstream patch
03f9751 
Apply upstream patch to fix a regression in 5.9.2.

Refs:
wolfSSL/wolfssl#10790
wolfSSL/wolfssl#10793
wolfSSL/wolfssl@7dd269f

Ref: #22175 (local mitigation attempt)
Fixes #22160

Closes #22204
vszakats pushed a commit to curl/curl that referenced this pull request Jul 9, 2026
@icing @vszakats
GHA/macos: ignore test 311 results for Homebrew wolfSSL
7183bec 
Homebrew is distributing v5.9.2 at the time of this commit, which
contains known regression failing to parse the server certificate.
The fix has been merged upstream. Till it makes it into a release
and Homebrew, ignore test results for 311.

Refs:
wolfSSL/wolfssl#10793
wolfSSL/wolfssl@7dd269f

Follow-up to 03f9751 #22204 #22175 #22160
Closes #22269
@vszakats vszakats mentioned this pull request Jul 9, 2026
Closed
vszakats added a commit to curl/curl that referenced this pull request Jul 9, 2026
@vszakats
tests: skip test 311 for wolfSSL 5.9.2
50c244c 
To fix this for everyone running curl tests.

Also: drop workarounds for CI.

Refs:
wolfSSL/wolfssl#10793
wolfSSL/wolfssl@7dd269f

Ref: #22269 (comment)
Follow-up to 7183bec #22269
Follow-up to 03f9751 #22204

Closes #22286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left review comments
@douzzer douzzer douzzer approved these changes

Assignees

@wolfSSL-Bot wolfSSL-Bot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: too generic return code for cert name mismatch

5 participants

@embhorn @douzzer @wolfSSL-Fenrir-bot @wolfSSL-Bot